Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[shared_preferences_foundation] Add Swift runtime search paths for Objective-C apps - #6952

Merged
jmagman merged 2 commits into
flutter-team-archive:mainfrom
jmagman:sp-swift
Jan 13, 2023
Merged

[shared_preferences_foundation] Add Swift runtime search paths for Objective-C apps#6952
jmagman merged 2 commits into
flutter-team-archive:mainfrom
jmagman:sp-swift

Conversation

@jmagman

@jmagman jmagman commented Jan 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

shared_preferences_foundation is using Swift as of #6940. The Objective-C projects do not automatically set up the search paths to find the Swift runtimes. Hook that up in the shared_preferences_foundation podspec.

See explanation at flutter/flutter#16049 (comment) and the first time this was done in plugins: #6369

Convert the example project back to Objective-C to catch future versions of this issue.

Fixes flutter/flutter#118418

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@jmagman jmagman self-assigned this Jan 13, 2023
jmagman
jmagman commented Jan 13, 2023
View reviewed changes
Comment thread ...hared_preferences/shared_preferences_foundation/example/ios/Runner.xcodeproj/project.pbxproj
97C147011CF9000F007C117D /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FF1CF9000F007C117D /* LaunchScreen.storyboard */; };
9AF3A5CAB88B27F2BC36D686 /* Pods_RunnerTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D4C443E7E16FB2AD978AC1D6 /* Pods_RunnerTests.framework */; };
B81650923B266CE1F32B75E4 /* Pods_Runner.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = F3702C135032CE4599D8327B /* Pods_Runner.framework */; };
F7E47F772970EBA900FA52F3 /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 7AFFD8EE1D35381100E5BB4D /* AppDelegate.m */; };

@jmagman jmagman Jan 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of reverting the project file back to before #6940 I instead surgically modified the project file to do the minimum to convert it back to an Objective-C app using static library versions of the plugin.

Comment thread ...hared_preferences/shared_preferences_foundation/darwin/shared_preferences_foundation.podspec
Comment on lines +22 to +23
'LIBRARY_SEARCH_PATHS' => '$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)/ $(SDKROOT)/usr/lib/swift',
'LD_RUNPATH_SEARCH_PATHS' => '/usr/lib/swift',

@jmagman jmagman Jan 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix.

@jmagman
jmagman marked this pull request as ready for review January 13, 2023 01:58
@jmagman
jmagman requested a review from cyanglaz as a code owner January 13, 2023 01:58
jmagman
jmagman commented Jan 13, 2023
View reviewed changes
Comment thread packages/shared_preferences/shared_preferences_foundation/example/ios/RunnerTests/RunnerTests.m
@implementation SharedPreferencesTests

- (void)testPlugin {
SharedPreferencesPlugin *plugin = [[SharedPreferencesPlugin alloc] init];

@jmagman jmagman Jan 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is pretty lame, I didn't re-write the better Swift tests back to Objective-C. Will do that after this lands, to unblock the tree ASAP.

@stuartmorgan-g stuartmorgan-g Jan 13, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to abandon the Swift tests? I would have thought we could have Swift tests even though the app is Objective-C.

@jmagman jmagman Jan 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that does the automagical thing with the Swift runtimes when running an XCTest since Xcode does all kinds of voodoo to get the tests to see non-public APIs? Relying on this from the example app seems fragile though, it's just luck that we caught it in the storekit example app. I'll try to think about a better way to holistically test this problem...

stuartmorgan-g
stuartmorgan-g approved these changes Jan 13, 2023
View reviewed changes

@stuartmorgan-g stuartmorgan-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to fix the regression; one question about the test follow-up.

@stuartmorgan-g

stuartmorgan-g commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

(Thanks for cleaning this up, and sorry I ignored the warning sign of the CI issue when I switched the app-facing package.)

@jmagman

jmagman commented Jan 13, 2023

Copy link
Copy Markdown
Contributor Author

Merging. @stuartmorgan to publish tomorrow, which should green out the tree.

@jmagman
jmagman merged commit be2e3de into flutter-team-archive:main Jan 13, 2023
@jmagman
jmagman deleted the sp-swift branch January 13, 2023 04:10
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 13, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2023
@engine-flutter-autoroll
be2e3de7a [shared_preferences_foundation] Add Swift runtime search pa…
dd40504 
…ths for Objective-C apps (flutter-team-archive/plugins#6952)
auto-submit Bot pushed a commit to flutter/flutter that referenced this pull request Jan 13, 2023
@engine-flutter-autoroll
Roll Plugins from 39197f17ca59 to 92a5367d58df (4 revisions) (#118457)
fb1a151 
* b89e4fc2d Roll Flutter from 7ddf42e to 0d91c03 (58 revisions) (flutter-team-archive/plugins#6948)

* 86eda6992 [path_provider] Switch to Pigeon for macOS (flutter-team-archive/plugins#6635)

* be2e3de7a [shared_preferences_foundation] Add Swift runtime search paths for Objective-C apps (flutter-team-archive/plugins#6952)

* 92a5367d5 [tool] Fix false positives in update-exceprts (flutter-team-archive/plugins#6950)
@stuartmorgan-g stuartmorgan-g mentioned this pull request Jan 13, 2023
Merged
11 tasks
auto-submit Bot pushed a commit that referenced this pull request Jan 13, 2023
@stuartmorgan-g
[shared_preferences] Revert recent iOS example changes (#6955)
f80aabf 
* Revert "[shared_preferences_foundation] Add Swift runtime search paths for Objective-C apps (#6952)"

This reverts commit be2e3de.

* Revert "[shared_preferences] Switch to `shared_preferences_foundation` (#6940)"

This reverts commit 44098fe.

* Re-apply the non-example parts of the reverted PRs
esouthren pushed a commit to esouthren/flutter that referenced this pull request Jan 15, 2023
@engine-flutter-autoroll @esouthren
Roll Plugins from 39197f17ca59 to 92a5367d58df (4 revisions) (flutter…
77f5ce0 
…#118457)

* b89e4fc2d Roll Flutter from 7ddf42e to 0d91c03 (58 revisions) (flutter-team-archive/plugins#6948)

* 86eda6992 [path_provider] Switch to Pigeon for macOS (flutter-team-archive/plugins#6635)

* be2e3de7a [shared_preferences_foundation] Add Swift runtime search paths for Objective-C apps (flutter-team-archive/plugins#6952)

* 92a5367d5 [tool] Fix false positives in update-exceprts (flutter-team-archive/plugins#6950)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cyanglaz cyanglaz Awaiting requested review from cyanglaz
1 more reviewer
@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jmagman jmagman

Labels

p: shared_preferences platform-ios

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

in_app_purchase_storekit iOS platform tests failing on master, crashing during native tests

2 participants

@jmagman @stuartmorgan-g